Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ci/GitHub actions #30

Merged
merged 29 commits into from
Jul 23, 2021
Merged

Ci/GitHub actions #30

merged 29 commits into from
Jul 23, 2021

Conversation

mkolopanis
Copy link
Contributor

@mkolopanis mkolopanis commented Jul 29, 2020

set up tests to also run on github actions.
closes #24
TODO:

  • coverage
  • Manifest.in ?

@mkolopanis
Copy link
Contributor Author

using this tox-gh-actions separates tests into different gh runners, which helps a little with speed. However now there is an annoying amount of actions. I'm not really tied to using it if you'd prefer to not.

@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #30 (825eb3a) into master (0ba16a7) will increase coverage by 0.60%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #30      +/-   ##
==========================================
+ Coverage   91.02%   91.63%   +0.60%     
==========================================
  Files          10       10              
  Lines         680      729      +49     
==========================================
+ Hits          619      668      +49     
  Misses         61       61              
Impacted Files Coverage Δ
py21cmsense/cli.py 97.18% <0.00%> (+0.08%) ⬆️
py21cmsense/sensitivity.py 93.63% <0.00%> (+0.39%) ⬆️
py21cmsense/observation.py 95.04% <0.00%> (+1.00%) ⬆️
py21cmsense/beam.py 85.45% <0.00%> (+1.14%) ⬆️
py21cmsense/observatory.py 84.30% <0.00%> (+1.28%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0ba16a7...825eb3a. Read the comment docs.

Copy link
Contributor

@steven-murray steven-murray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable, thanks Matt

@mkolopanis
Copy link
Contributor Author

Looks like pre-commit is having a lot of trouble on windows. but I can't find anything in tox to like conditionally not do pre-commit on windows, or how to fix this issue.

@mkolopanis mkolopanis marked this pull request as ready for review November 19, 2020 17:36
@steven-murray
Copy link
Contributor

@mkolopanis I would actually separate the pre-commit stuff from tox altogether -- make it a separate github action.

@mkolopanis
Copy link
Contributor Author

So currently it is run by tox as what is called flake8 do you mean you'd have another action in the tox suite that is just pre-commit. Or a single github action that only runs pre-commit (presumably on a single OS?) for each push/pr?

@steven-murray
Copy link
Contributor

Yeah, I'd take the flake8 out of tox, and add a separate github action just to run pre-commit.

@mkolopanis
Copy link
Contributor Author

Oh I changed the name to remove the reference to "Tox". Looks like you'll have to update the "required checks" since those will never run again.

@steven-murray
Copy link
Contributor

🎉

@steven-murray steven-murray merged commit 109bdb1 into master Jul 23, 2021
@steven-murray steven-murray deleted the ci/github_actions branch July 23, 2021 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move to github actions
2 participants